Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use tag instead of directory based metadata #177

Merged
merged 20 commits into from
Nov 20, 2023
Merged

Use tag instead of directory based metadata #177

merged 20 commits into from
Nov 20, 2023

Conversation

NattyNarwhal
Copy link
Member

@NattyNarwhal NattyNarwhal commented Nov 9, 2023

Avoids the ugly contorting of the data model from arbitrary directory structure to artist-album-track. This should make the API requests actually match the original data model.

  • I have only tested with Navidrome. Since Navidrome uses identical IDs for the fake hierarchy and the tags, the transition is seamless.
  • I haven't tested Subsonic. Since it uses different IDs for directories vs. tags, it's likely this won't be so smooth. Recreate your server. (Could this be handled better?)
  • Nor have I tested other implementations.
  • Podcasts may be busted. The podcast functionality is really bitrotted and not well tested regardless.
  • We could do real directory hierarchies (Feature request: View by folder (and other options) #173) correctly, with a data model actually modelling... directories.

Fixes GH-73

Avoids the ugly contorting of the data model from arbitrary directory
structure to artist-album-track. This should make the API requests
actually match the original data model.

- I have only tested with Navidrome. Since Navidrome uses identical IDs
  for the fake hierarchy and the tags, the transition is seamless.
- I haven't tested Subsonic. Since it uses different IDs for directories
  vs. tags, it's likely this won't be so smooth. Recreate your server.
  (Could this be handled better?)
- Nor have I tested other implementations.
- Podcasts may be busted. The podcast functionality is really bitrotted
  and not well tested regardless.
- We could do real directory hierarchies (GH-173) correctly, with a data
  model actually modelling... directories.
The first element is subtly different for getAlbumList(2)
@NattyNarwhal
Copy link
Member Author

NattyNarwhal commented Nov 9, 2023

I'd like your help testing this pull request. Let me know if it works for your server (or not), and if you run into any issues. I've only tested this against Navidrome so far; I'd like to know how it works for the other questions raised in the description for this PR.

Backup your database before doing anything. At minimum, the Submariner Library.sqlite files, and the whole Submariner directory if you want to play it safe. This version includes a database schema upgrade, so you may not be able to revert back to a previous version until you restore the database (or reset it).

Although it identifies as "2.4.2", it includes all the changes made since then intended for 2.5, as well the changes in this PR. If you have issues or questions regarding those unrelated changes, file a separate issue.


(This comment will be updated as needed)

Download here 2e7318e

Download here Older, 5591c09

Download here Older, fc64db5

It gets a bit wobbly looking if there's some irregular entries and
albums get cleared out via item not found errors popping up, but it does
make migration surprisingly realistic.
The podcast stuff is still using one of them though...
Reset IDs, names, and paths when possible. Better handle object
ownership when possible.
It returned application/x-download, which would confuse the logic. It
shouldn't really be relying on file extensions anyways, but the content
of the files. (TagLib when?)

Not strictly needed for ID3 work, can be cherry picked back to master
...since they'll always be invalid. Will clear up entries in a Subsonic
directory index to ID3 transition, better than empty set would. Also
purges the name-only entries too!
If we do directory support again, we should handle it correctly.

Maybe we can clean up the request types?

Some of it can't be removed until podcasts, but it seems podcasts are
going to be really crumbly until that...
@NattyNarwhal
Copy link
Member Author

NattyNarwhal commented Nov 11, 2023

Build coming soon with the changes pushed since the release, but I've been testing Subsonic and trying to handle migration correctly:

  • Podcast support will probably be more broken until correct raw directory support is done, since podcast APIs hard rely on parent. (That'll involve at least schema changes, plus UI for it.) That, or maybe just call getSong on the streamID if it works for a podcast episode's file? Maybe podcast support can be removed, unless someone's using it?
  • Artist and albums in the conversion are still a bit wonky. The first reordering of the library can be confusing, but switching from the server and back can fix that. Disassociated albums might appear and get code 70'd (with a message box), but sorts itself out for that artist after that.

This was never working in the first place. The podcast menu item didn't
switch, and items were duplicating. And the stream URL was for the wrong
ID type. Things now work, so it's at least better than it used to be.

Still some grungy stuff - there's no way to manipulate the podcasts, for
instance. And the automatic download stuff has some curious ideas about
metadata? Lots of UI thoughts for anywhere where podcasts intersect w/
noormal tracks.

(Or maybe we can remove it?)
@NattyNarwhal
Copy link
Member Author

Podcast support was really broken before, but now it's... limping, with some UX confusion going on. It might be fixable, but it'll have to block on this branch.

I think it's because in Subsonic, the album objects can't be rated, but
directories can be. Weh.
When transitioning to the new schema, you'll likely get albums that
don't exist, though we try to update as soon as possible when the artist
album list is pulled. My guess is a race condition between getting the
album list from the server and the cached albums from Core Data.

The easiest workaround is to just not show this message.
There were some uh, nasty playlists on demo server that got cleaned up
remotely, but not locally
@NattyNarwhal
Copy link
Member Author

I think I'll just go ahead with this and consider this a change that might merit a 3.0.

@NattyNarwhal NattyNarwhal merged commit 2e7318e into master Nov 20, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use metadata based index instead of filesystem in queries
1 participant